refactor: shrink the renderer's first-load chunk and remove three duplicated authorities - #4101
Conversation
jackwener
left a comment
jackwener
left a comment
There was a problem hiding this comment.
I reviewed exact head 0309897ef047d7897c5bf57674d32fb156b642c2.
Workbar no longer re-exports WorkbarSurface from the production barrel, so the lazy import can actually split. Assistant and thinking streams share one redact-append-cap pipeline and keep their own caps and head/tail recovery. ComposerProps comes from @maka/ui. Roving-row tabIndex is written only when the value changes.
No blocking issues. One minor note inline: the new shared pipeline has no tests of its own.
Posted by an automated review agent operated by @WAWQAQ. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong.
简体中文
我审了当前 head。没有阻塞问题。一处可延后：共用流式管道没有自己的测试。
本条评论由 @WAWQAQ 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查，也不能替代人类审查。有人类对本条评论负责，如有错误请直接指出。
| * The carried state is opaque: the live projection stores only a WeakMap key | ||
| * and length counters, never the raw mutable suffix as enumerable React state. | ||
| */ | ||
| export function applyStreamDelta( |
There was a problem hiding this comment.
Minor: this is the new shared pipeline, but the tests still only go through applyAssistantDelta / applyThinkingDelta. If head-keep and tail-keep later diverge here, those wrappers may not fail first. Not blocking.
简体中文
可延后：共用管道没有直接测试，现在只覆盖两条包装函数。
The feature barrel statically re-exported `WorkbarSurface`, and `app-shell` imports `WorkbarHost` from that barrel, so the surface and its five nested lazy tool panels landed in the eager chunk anyway. rolldown named it: INEFFECTIVE_DYNAMIC_IMPORT. Drop it from the barrel and give Storybook its own public entry. It cannot share `testing`: that module is loaded by `node --test` against tsc output, and the surface and its tool panels use extensionless relative specifiers only a bundler resolves, so re-exporting it there breaks every node suite behind `testing` at load time. Stories run through Vite and have no such limit. `workbar-boundary` recognizes the new entry alongside the other two. Renderer entry chunk 2,245.33 kB -> 2,166.20 kB (gzip 451.12 -> 426.86), with `workbar-surface` now emitted as its own 81.31 kB chunk, and the build warning is gone.
Four workbar files reached `ComposerProps` through a seven- or eight-level relative path into `packages/ui/dist/composer.d.ts`, a build artifact. `@maka/ui` already exports the type, and all four files already import from it.
0309897 to
46c0c0d
Compare
`assistant-stream` and `thinking-stream` were two copies of one authority: identical Options/Result shapes, the same five-step redact-append-cap pipeline, the same non-string guard, structurally identical complete paths. They differed only in caps, marker strings, and which end of an over-cap buffer survives — a direction `streaming-display-redaction` already parameterizes as `recovery`. `stream-delta` now owns the pipeline; the two modules keep their exported names, caps, and option types as thin wrappers that supply their own spec. Head-keep's short-circuit on a frozen buffer is now expressed as a property of `recovery: 'head'` rather than of the assistant stream, which is what it always was. Tested directly rather than only through the wrappers, since each wrapper exercises one direction and neither would fail first if the two stopped differing where they must. A mutation that routes head-keep through the tail-keep branch fails the new suite. `tool-output-stream` stays separate: it accumulates a chunk array with dedup-by-seq, a different problem.
The first two render-phase branches reseeded `activation` with a byte-identical object literal, so they are one condition. The third branch clears `initialLiveContent` and stays as it is. The snapshot passed to `ChatView` also dropped its session guard: every branch reseeds `sessionId` to `activeSessionId`, and a render-phase setState re-runs the component body before anything commits, so the mismatched arm could never reach the DOM.
The layout effect deliberately runs on every render, and it wrote `tabIndex` on every row each time. An identical assignment still invalidates style and forces synchronous layout — DevTools charged the loop 224ms of reflow across the module pages that use it. Compare before assigning. A freshly mounted row still gets its tabindex, because it arrives at the default 0.
46c0c0d to
b13019d
Compare
|
Addressed the note on Nine tests against
Verified the suite actually bites: routing head-keep through the tail-keep branch fails it (8/9), and the tree is back to 9/9 with that mutation reverted. Note that the approval above is against |
#4101 gave Storybook its own `features/workbar/stories` entry and moved `WorkbarSurface` there, but the feature README still said stories share `testing`. Name all three entries and why the split exists, so the next reader does not re-derive it from `workbar-boundary.test.ts`.
2 participants
Five simplification-audit findings in the desktop renderer and
packages/ui. All internal; no user-visible behavior changes.This is not a fix for the session-switch jank. The measured session-switch cost (9 long tasks, 717 ms, CLS 0.55) has a different, still-unidentified cause and is tracked separately. What is here is bundle size, one duplicated authority removed, and two boundary escape hatches closed.
Bundle size
The workbar feature barrel statically re-exported
WorkbarSurfacewhileworkbar-hostreaches it throughlazy(() => import('./workbar-surface')). Becauseapp-shellimportsWorkbarHostfrom that barrel, the static edge dragged the 917-line surface and its five nested tool panels into the eager chunk — rolldown reportedINEFFECTIVE_DYNAMIC_IMPORT.Two Storybook files did import
WorkbarSurfacefrom the barrel, so the feature gains a Storybook-only public entry,features/workbar/stories. It cannot sharetesting: that module is loaded bynode --testagainst tsc output, and the surface and its tool panels use extensionless relative specifiers that only a bundler resolves, so re-exporting it there breaks every node suite behindtestingat load time. Stories run through Vite.workbar-boundary.test.tsrecognizes the new entry alongsideindexandtesting; nothing that ships imports it.workbar-surfacechunkINEFFECTIVE_DYNAMIC_IMPORTOne duplicated authority
assistant-streamandthinking-streamwere two implementations of one concept — identical Options/Result shapes, the same redact-append-cap pipeline, the same non-string guard, structurally identical complete paths. They differed only in caps, marker strings, and which end of an over-cap buffer survives, andstreaming-display-redactionalready parameterizes that asrecovery: 'head' | 'tail'.stream-deltanow owns the pipeline. Both modules keep their exported names, cap constants, and option types as thin wrappers. Head-keep's short-circuit on a frozen buffer is now expressed as a property ofrecovery: 'head', which is what it always was — tail-keep has no fixed point, so it is unaffected.stream-deltais tested directly, not only through the wrappers: each wrapper exercises onerecoverydirection, so neither would fail first if the two stopped differing where they must. A mutation that routes head-keep through the tail-keep branch fails the new suite.tool-output-streamis deliberately left alone: it accumulates a chunk array with dedup-by-seq, a different problem.Two boundary escape hatches
Four workbar files imported
ComposerPropsthrough a seven- or eight-level relative path intopackages/ui/dist/composer.d.ts, a build artifact.@maka/uiexports the type and all four already import from it.Two local simplifications
chat-message-surface: the first two render-phase activation branches calledsetActivationwith a byte-identical literal — now one condition. The snapshot handed toChatViewalso dropped a session guard whose else branch was unreachable: every branch reseedssessionIdtoactiveSessionId, and a render-phase setState re-runs the body before anything commits.use-roving-row-focus: the dependency-array-free layout effect (deliberate, and left that way) wrotetabIndexon every row on every render. An identical write still invalidates style and forces synchronous layout — DevTools charged it 224 ms of reflow. It now compares first. This helps the module pages that use the hook (skills, scheduled tasks, MCP); the chat transcript does not use it.Verification
npm run format,npm run check:asf-headers,biome lintover the touched trees@maka/uibuild + fullpackages/uitest suite — 256/256, including the newstream-deltateststsconfig.renderer.jsonandtsconfig.storybook.jsontypechecksapps/desktop/src/main/__tests__/{assistant,thinking}-stream.test.ts— 12/12vite buildof the renderer, A/B against the same tree with the barrel export restoredapps/desktopnode suite offbuild:main— 1640/1640, includingworkbar-boundary.test.ts